Finish milestone #178
Finish milestone #178
Conversation
# Conflicts: # gh_review_project/review_project.py # gh_review_project/test/test.json
james-bruten-mo
left a comment
james-bruten-mo
left a comment
There was a problem hiding this comment.
Just a couple of suggestions
| import shlex | ||
| from collections import defaultdict | ||
|
|
||
| project_id = 376 |
There was a problem hiding this comment.
| project_id = 376 | |
| PROJECT_ID = 376 |
| from collections import defaultdict | ||
|
|
||
| project_id = 376 | ||
| project_owner = "MetOffice" |
There was a problem hiding this comment.
| project_owner = "MetOffice" | |
| PROJECT_OWNER = "MetOffice" |
| raise RuntimeError( | ||
| "Error fetching GitHub Project data: \n " + output.stderr.decode() | ||
| ) | ||
| command = f"gh project item-list {project_id} -L 500 --owner {project_owner} --format json" |
There was a problem hiding this comment.
It feels like project_id/owner should be inputs to this function, rather than hard-coded to the global variable.
Maybe def from_github(cls, project_id=PROJECT_ID, project_owner=PROJECT_OWNER...)?
There was a problem hiding this comment.
given there is much in this code that is specific to this project, I've left it just pulling directly from the globals for now since there are bigger changes needed to make this an option.
| """ | ||
|
|
||
| data = defaultdict(list) | ||
| data = [] |
There was a problem hiding this comment.
Would pull_requests or pull_requests_data be a clearer name?
There was a problem hiding this comment.
I guess same with self.data? If it's not just pr's then fair enough
| dry_run: If true, print the command used rather than archiving. | ||
| """ | ||
|
|
||
| command = f"gh project item-archive {project_id} --owner {project_owner} --id {self.id}" |
There was a problem hiding this comment.
Similar comments around project_owner/id as above
| pr.archive(dry_run) | ||
|
|
||
|
|
||
| class PullRequest: |
There was a problem hiding this comment.
Probably wants a docstring
| self.scitechReview = None | ||
| self.codeReview = None | ||
|
|
||
| def archive(self, dry_run: bool = False): |
There was a problem hiding this comment.
| def archive(self, dry_run: bool = False): | |
| def archive(self, dry_run: bool = False) -> None: |
| exceptions. | ||
| """ | ||
| total_open = still_open(open_prs[milestone], milestone) | ||
| total_other = closed_other(closed_prs, milestone) |
There was a problem hiding this comment.
Do we want to update the milestone on these PRs to match the closing milestone? I guess you'd logically do it after the check_ready step and before report?
| * Remaining open PRs and issues against this milestone | ||
| * Closed PRs against this milestone | ||
| """ | ||
| from collections import defaultdict |
There was a problem hiding this comment.
Not sure you're using this in this file?
2 participants
PR Summary
Sci/Tech Reviewer:
Code Reviewer: @james-bruten-mo
New script for closing a milestone from the Review Tracker project during a release. It:
To do this it also improves the project data class, simplifying the data structure, including functions for handling milestones and storing the PR details in objects rather than a dictionary.
The user will need to authenticate themselves to modify the project, github prompts for this on first use.
Code Quality Checklist
Testing
Security Considerations
AI Assistance and Attribution
Sci/Tech Review
(Please alert the code reviewer via a tag when you have approved the SR)
Code Review